Skip to content

Conversation

compiler-errors
Copy link
Member

The checks that we do in the constness query seem inconsistent with the checks that we do to determine if an item's param-env is const, so I merged them into the constness query and call that from the param_env query.

I'm not sure if this totally makes sense -- is there a case where tcx.param_env() would return a const param-env for an item whose tcx.constness() is Constness::NotConst? Because if not, it seems a bit dangerous that these two differ.

Luckily, not many places actually use tcx.constness(), and the checks in tcx.param_env() seem stricter than the checks in tcx.constness() (at least for the types of items we type-check).

Also, due to the way that tcx.param_env() is implemented, it never used to return a const param-env for a item coming from a different crate, which also seems dangerous (though also probably not weaponizable currently, because we seldom actually compute the param-env for a non-local item).

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 9, 2022
@rust-highfive
Copy link
Contributor

r? @petrochenkov

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2022
@compiler-errors
Copy link
Member Author

r? @fee1-dead since I think I saw from git-blame that you touched this last, though maybe also cc @oli-obk for const+types related stuff.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 9, 2022

After #101900 we'll be able to remove the constness from ParamEnv entirely, so not sure it's worth it doing anything with it. But seems fine to me either way.

@fee1-dead
Copy link
Member

LGTM

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 9, 2022

📌 Commit 01d566c8cb73c69d237d7fb41b2119d1f69efa8d has been approved by fee1-dead

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 9, 2022
@bors
Copy link
Collaborator

bors commented Oct 9, 2022

☔ The latest upstream changes (presumably #102850) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 9, 2022
@fee1-dead
Copy link
Member

r=me once ci green

@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member Author

@bors r=fee1-dead

@bors
Copy link
Collaborator

bors commented Oct 10, 2022

📌 Commit c1c159f has been approved by fee1-dead

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 10, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 10, 2022
…r=fee1-dead

Unify `tcx.constness` query and param env constness checks

The checks that we do in the `constness` query seem inconsistent with the checks that we do to determine if an item's param-env is const, so I merged them into the `constness` query and call that from the `param_env` query.

I'm not sure if this totally makes sense -- is there a case where `tcx.param_env()` would return a const param-env for an item whose `tcx.constness()` is `Constness::NotConst`? Because if not, it seems a bit dangerous that these two differ.

Luckily, not many places actually use `tcx.constness()`, and the checks in `tcx.param_env()` seem stricter than the checks in `tcx.constness()` (at least for the types of items we type-check).

Also, due to the way that `tcx.param_env()` is implemented, it _never_ used to return a const param-env for a item coming from a different crate, which also seems dangerous (though also probably not weaponizable currently, because we seldom actually compute the param-env for a non-local item).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 10, 2022
…r=fee1-dead

Unify `tcx.constness` query and param env constness checks

The checks that we do in the `constness` query seem inconsistent with the checks that we do to determine if an item's param-env is const, so I merged them into the `constness` query and call that from the `param_env` query.

I'm not sure if this totally makes sense -- is there a case where `tcx.param_env()` would return a const param-env for an item whose `tcx.constness()` is `Constness::NotConst`? Because if not, it seems a bit dangerous that these two differ.

Luckily, not many places actually use `tcx.constness()`, and the checks in `tcx.param_env()` seem stricter than the checks in `tcx.constness()` (at least for the types of items we type-check).

Also, due to the way that `tcx.param_env()` is implemented, it _never_ used to return a const param-env for a item coming from a different crate, which also seems dangerous (though also probably not weaponizable currently, because we seldom actually compute the param-env for a non-local item).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 11, 2022
…r=fee1-dead

Unify `tcx.constness` query and param env constness checks

The checks that we do in the `constness` query seem inconsistent with the checks that we do to determine if an item's param-env is const, so I merged them into the `constness` query and call that from the `param_env` query.

I'm not sure if this totally makes sense -- is there a case where `tcx.param_env()` would return a const param-env for an item whose `tcx.constness()` is `Constness::NotConst`? Because if not, it seems a bit dangerous that these two differ.

Luckily, not many places actually use `tcx.constness()`, and the checks in `tcx.param_env()` seem stricter than the checks in `tcx.constness()` (at least for the types of items we type-check).

Also, due to the way that `tcx.param_env()` is implemented, it _never_ used to return a const param-env for a item coming from a different crate, which also seems dangerous (though also probably not weaponizable currently, because we seldom actually compute the param-env for a non-local item).
@compiler-errors
Copy link
Member Author

@bors r- failed in rollup

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Oct 11, 2022
@fee1-dead
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 12, 2022

📌 Commit bef8681 has been approved by fee1-dead

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 12, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Oct 12, 2022
…r=fee1-dead

Unify `tcx.constness` query and param env constness checks

The checks that we do in the `constness` query seem inconsistent with the checks that we do to determine if an item's param-env is const, so I merged them into the `constness` query and call that from the `param_env` query.

I'm not sure if this totally makes sense -- is there a case where `tcx.param_env()` would return a const param-env for an item whose `tcx.constness()` is `Constness::NotConst`? Because if not, it seems a bit dangerous that these two differ.

Luckily, not many places actually use `tcx.constness()`, and the checks in `tcx.param_env()` seem stricter than the checks in `tcx.constness()` (at least for the types of items we type-check).

Also, due to the way that `tcx.param_env()` is implemented, it _never_ used to return a const param-env for a item coming from a different crate, which also seems dangerous (though also probably not weaponizable currently, because we seldom actually compute the param-env for a non-local item).
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#102623 (translation: eager translation)
 - rust-lang#102719 (Enforce alphabetical sorting with tidy)
 - rust-lang#102830 (Unify `tcx.constness` query and param env constness checks)
 - rust-lang#102883 (Fix stabilization of `feature(half_open_range_patterns)`)
 - rust-lang#102927 (Fix `let` keyword removal suggestion in structs)
 - rust-lang#102936 (rustdoc: remove unused CSS `nav.sum`)
 - rust-lang#102940 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c763ebc into rust-lang:master Oct 12, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 12, 2022
albertlarsan68 added a commit to albertlarsan68/rust that referenced this pull request Oct 14, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#102623 (translation: eager translation)
 - rust-lang#102769 (Clean up rustdoc startup)
 - rust-lang#102830 (Unify `tcx.constness` query and param env constness checks)
 - rust-lang#102847 (impl AsFd and AsRawFd for io::{Stdin, Stdout, Stderr}, not the sys versions)
 - rust-lang#102883 (Fix stabilization of `feature(half_open_range_patterns)`)
 - rust-lang#102936 (rustdoc: remove unused CSS `nav.sum`)
 - rust-lang#102940 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 26, 2022
Revert "Unify tcx.constness and param env constness checks"

Too much of a perf regression rust-lang#102975 (comment), and an attempt in rust-lang#103263 didn't fix it except for just a tiny bit.

This change isn't really needed (see rust-lang#102830 (comment)), so this should be an easy revert.
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Oct 30, 2022
Revert "Unify tcx.constness and param env constness checks"

Too much of a perf regression rust-lang/rust#102975 (comment), and an attempt in #103263 didn't fix it except for just a tiny bit.

This change isn't really needed (see rust-lang/rust#102830 (comment)), so this should be an easy revert.
@compiler-errors compiler-errors deleted the constness-parity branch November 2, 2022 02:54
Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request Jan 6, 2023
Rollup of 7 pull requests

Successful merges:

 - rust-lang#102623 (translation: eager translation)
 - rust-lang#102719 (Enforce alphabetical sorting with tidy)
 - rust-lang#102830 (Unify `tcx.constness` query and param env constness checks)
 - rust-lang#102883 (Fix stabilization of `feature(half_open_range_patterns)`)
 - rust-lang#102927 (Fix `let` keyword removal suggestion in structs)
 - rust-lang#102936 (rustdoc: remove unused CSS `nav.sum`)
 - rust-lang#102940 (Update books)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants